Typed HttpMethodOverride Middleware #2407
Conversation
| @@ -15,23 +15,23 @@ class HttpMethodOverriderSpec extends Http4sSpec { | |||
| private final val varyHeader = "Vary" | |||
| private final val customHeader = "X-Custom-Header" | |||
|
|
|||
| private val headerOverrideStrategy = HeaderOverrideStrategy(CaseInsensitiveString(overrideHeader)) | |||
| private val queryOverrideStrategy = QueryOverrideStrategy(overrideParam) | |||
| private def headerOverrideStrategy[F[_], G[_]] = HeaderOverrideStrategy[F, G](CaseInsensitiveString(overrideHeader)) | |||
There was a problem hiding this comment.
Do these F and G infer? We may have imposed a slight usability tax here. I'm for it anyway if it increases soundness, but it'd be really neat if it didn't make it harder to use. It's a little hard for me to visualize the impact with the strategies all defined at the top like this.
There was a problem hiding this comment.
They do not infer nicely, Not really sure what I can do to improve it and keep the type inference we have however.
There was a problem hiding this comment.
I think it's still a better solution than reflection.
There was a problem hiding this comment.
I agree too.
BTW, I just find out about this now. @ChristopherDavenport next time please take some time to ask if it would be nice to apply the changes on your own. I had your suggestions almost ready for a couple of days now but I was waiting to get back to my personal computer after being AFK for some days due to personal matters.
Don't misunderstand me, I appreciate your review, suggestions and the work done to push this into master but, imagine if I'd expend more than just a few minutes on that. 😉
Anyways, thanks for everything and, apologies for arriving late to the party.
There was a problem hiding this comment.
@nebtrx I apologize if I rushed things. We've been working up to another milestone release and when I did not hear back I pushed forward as I wanted this to make the release.
It was not my intention to undercut your efforts and truly appreciate all the time and thought it took for you to put this together.
I hope this will not discourage you from contributing in the future. I am sorry, and will do my best moving forward to wait for additional feedback.
There was a problem hiding this comment.
Nah, it's okay. I get it. I truly appreciate the work you guys are doing and you can bet I'll be contributing again.
Updates with types on #2355
Otherwise the same PR